Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a few new metadata variables in tempo for QC and BC #1597

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

jeromebarre
Copy link
Contributor

@jeromebarre jeromebarre commented Dec 27, 2024

Description

Add a few new metadata variables in tempo for QC and BC.
EDIT:
This PR now test TEMPO for V3 products. Previous version are deprecated and shouldn't be used. The version option has been removed.

The PR now adds solar zenith angle, viewing zenith angle and albedo with the correct convention format in metadata for both TropOMI and TEMPO products.
Those quantities will be most helpful for future bias correction.

Issue(s) addressed

Resolves #<issue_number>

Dependencies

List the other PRs that this PR is dependent on:

  • ...

Impact

Expected impact on downstream repositories:

Checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have run the unit tests before creating the PR

@@ -278,6 +296,9 @@ def _read(self):
self.outdata[('longitude', 'MetaData')] = lons[flg]
self.outdata[('quality_assurance_value', 'MetaData')] = qa_value[flg]
self.outdata[('cloud_fraction', 'MetaData')] = cld_fra[flg]
self.outdata[('solar_zenith_angle', 'MetaData')] = sza[flg]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these the right IODA names? I thought snake case was frowned upon?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @CoryMartin-NOAA ,, you are correct the name to use is:
solarZenithAngle

https://github.com/JCSDA-internal/ioda/blob/202aafb341afdd667562ded96447abb34199bf43/share/ioda/yaml/validation/ObsSpace.yaml#L955

note the first name in the list is the current standard with the other names ones that would be changed into the leading entry if the upgrader were run something like:

${JEDI_BUILD}/bin/ioda-upgrade-v2-to-v3.x ${ioda_v2} ${ioda_v3} ${JEDI_SRC}/ioda/share/ioda/yaml/validation/ObsSpace.yaml

@jeromebarre jeromebarre added the needs review Asking others to review - often used for pull requests label Jan 30, 2025
@jeromebarre
Copy link
Contributor Author

@ashley314 there is no more version option in the TEMPO converter with that PR. We would need to update the EWOK and Skylab parts for the ingest suites.

@ashley314
Copy link
Contributor

@jeromebarre Thanks, I'll take a look and open a ticket for the ingest suite.

src/compo/tropomi_no2_co_nc2ioda.py Show resolved Hide resolved
foreach (spec no2) #hcho not tested and o3 not out yet with real data
if (NOT (${spec} STREQUAL "hcho" AND (${coltype} STREQUAL "total" OR ${coltype} STREQUAL "stratosphere")))
foreach (coltype total troposphere)
foreach (spec NO2) #hcho not tested and o3 not out yet with real data
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please make the indentation nicer?

src/compo/tempo_nc2ioda.py Outdated Show resolved Hide resolved
test/CMakeLists.txt Show resolved Hide resolved
@jeromebarre
Copy link
Contributor Author

I have addressed the review comments

@jeromebarre
Copy link
Contributor Author

@ashley314 the PR now uniformize the the column options names. It uses troposphere and total now. Befoe some of the converters were using tropo. I cleared this ambiguity but we will have to update the ingest scripts associated. See this commit 29544d5

@ashley314
Copy link
Contributor

@ashley314 the PR now uniformize the the column options names. It uses troposphere and total now. Befoe some of the converters were using tropo. I cleared this ambiguity but we will have to update the ingest scripts associated. See this commit 29544d5

Thank you, I made updates to update to troposphere in the ingest suite in https://github.com/JCSDA-internal/skylab/pull/673

@BenjaminRuston BenjaminRuston self-requested a review February 7, 2025 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
COMPO Atmospheric COMPOsition needs review Asking others to review - often used for pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants